Fix propagation of global attributes to child datasets and variables#72
Fix propagation of global attributes to child datasets and variables#72
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
forman
left a comment
There was a problem hiding this comment.
Please fix:
- It is not clear, why the fix is correct. New code is also quite unintuitive.
- Missing unit tests
| rule_op.validate_datatree(context, node) | ||
|
|
||
| if node.datatree.is_leaf: | ||
| # Inherit attrs from the parent datatree into the child dataset |
There was a problem hiding this comment.
This just explains what the code does, but this is obvious. Please answer the question why this is done. What issues is solved by copying attributes.
| ) | ||
| else: | ||
| for name, datatree in node.datatree.children.items(): | ||
| # Inherit attrs from the parent datatree into the child datatree |
| - Fixed passing of global datatree attributes to children: attributes defined | ||
| on parent datatrees are now inherited by all descendants. (#63) |
There was a problem hiding this comment.
Why? To me this sounds like an xarray bug for which we provide a workaround here. Why do we have to inherit them manually? To me it seems we are copying attributes from a parent group into a child group - that would be wrong as it does not reflect the true physical structure of the Zarr dataset.
This PR fixes passing of global datatree attributes to children. Attributes defined on parent datatrees are now inherited by all descendants.
Closes #63.
Checklist (strike out non-applicable):
CHANGES.mdCHANGES.mddocs/*